Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TIND ILS: Add 269, 720 fields #3413

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

skairunner
Copy link

@skairunner skairunner commented Jan 29, 2025

  • Include 720 in the list of fields considered as creators. It is used by e.g. Social Media Archive to list contributors.
  • Refactor MARCXML importer to expose a utility function that can be manually driven, similar to the MARC importer.
  • Use the MARCXML importer in TIND ILS to fetch dialect-specific non-standard MARC information, i.e. the use of 269 as a date.

This PR will remain a draft until #3409 is merged for two reasons:

  • The 720 creators should be contributors, but some code in the MARC importer converts all creators to editors if there is no author and item type is book. All TIND item types are currently set to book. The above PR fixes this.
  • All the old tests are broken and are fixed in Library Catalog (TIND ILS): Enrich item type from Schema.org #3409.

@AbeJellinek
Copy link
Member

#3409 is merged, so go ahead and rebase!

@skairunner
Copy link
Author

Will do, out sick for the moment so it might be a bit 😄

- 720 fields are sometimes used e.g. in datasets to describe additional
  contributors.
- Use 720 field as a contributor.
- Note that tests will fail until zotero#3409 is merged. This is because the
  MARC code assumes that all books with no authors that have
  contributors are actually meant to be editors, which is invalid.
  However, we have no way of setting items to be not Book without zotero#3409.
- Refactor MARCXML importer to expose a utility function, similar to
  the MARC importer. This will allow consumers of the MARCXML translator
  to instead decide to manually drive the process.
- Convert to using the MARCXML utility function directly
- Use the MARCXML utility to read date from the 269 field, which is a
  non-standard MARC field.
@skairunner
Copy link
Author

I have updated the branch. It seems CI fails because I modified MARC.js and MARCXML.js which are used by over 10 translators.

@skairunner skairunner marked this pull request as ready for review February 3, 2025 08:52
Copy link
Member

@AbeJellinek AbeJellinek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! This generally looks great. Some comments/suggestions below.

MARC.js Outdated Show resolved Hide resolved
MARC.js Outdated Show resolved Hide resolved
MARC.js Outdated Show resolved Hide resolved
MARC.js Outdated
"creators": [
{
"lastName": "Meta Platforms, Inc",
"creatorType": "contributor",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is failing because the translator returns editor, not contributor. I think editor is more correct, but the test needs to be updated.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, editor is what would be expected:

  • since the example MARC doesn't have a leader, it defaults to book
  • The MARC doesn't have any authors
  • There's some code in MARC.js that sets all creators to editor if a book has no authors

I don't know if more correct is what I'd say in terms of how MARC should behave, though.

MARCXML.js Show resolved Hide resolved
MARCXML.js Outdated Show resolved Hide resolved
Library Catalog (TIND ILS).js Outdated Show resolved Hide resolved
Library Catalog (TIND ILS).js Outdated Show resolved Hide resolved
Library Catalog (TIND ILS).js Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants